Match filter attributes by identity instead of equality#1584
Open
PranavMishra28 wants to merge 1 commit into
Open
Match filter attributes by identity instead of equality#1584PranavMishra28 wants to merge 1 commit into
PranavMishra28 wants to merge 1 commit into
Conversation
attrs.filters.include/exclude stored the passed Attribute instances in a frozenset and tested membership with `attribute in attrs`. Attribute equality ignores the owning class, so two identically-configured attributes on different classes compare equal - filtering by one class's attribute also matched the other class's. Keep the attributes in a tuple and match them by identity instead. Field-name strings still match by name across classes, which covers the cross-class use case. Fixes python-attrs#864 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author
|
just wanted to highlight that this is ready for a look whenever you have a moment. It directly fixes the equality bug in #864 and all 19 checks are passing green. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was broken
attrs.filters.include/excludeaccept types, field-name strings, andAttributeinstances. TheAttributeinstances were stored in afrozensetand matched withattribute in attrs, i.e. by equality. ButAttribute.__eq__/__hash__ignore the owning class, so two identically-configured same-named attributes on different classes compare equal:Excluding (or including) one class's attribute silently affects an identically-configured attribute of an unrelated class.
Why it matters
This is the behavior tracked in #864, which is labeled
Bug. As noted there,Attributeinstances are independent of the classes they're defined on, so equality-based matching "is absolutely not what people would expect."attr.fields(A).xandattr.fields(B).xare distinct objects, so identity is the correct test.What the fix does
_split_whatkeeps the passed attributes in atuple(instead of afrozenset), andinclude/excludematch them withany(attribute is a for a in attrs)— by identity. Sincefields()returns the same cachedAttributeobject for a given class, the intended attribute still matches, while an equal-but-distinct attribute on another class no longer does.What it deliberately does not change
exclude("repeated")), which matches by name across classes.include/excludesignatures and the.pyistub are unchanged.Note on compatibility
This narrows
Attributematching from equality to identity. Anyone who (perhaps unknowingly) relied on the equal-but-distinct cross-class match would see a behavior change; per the issue this was considered a bug rather than intended behavior, so I filed it underchange. Happy to reclassify asbreakingif you'd prefer.Testing
includeandexclude(equal-but-distinct attribute on another class is not matched); updated the_split_whattest for the tuple.1383 passed, 9 skipped, 1 xfailed;ruff checkandruff format --checkclean. Addedchangelog.d/864.change.md.Fixes #864
Developed with Claude Code; reviewed and tested by Pranav before marking ready for review.